EuiBottomBar: allow customization of whether the component makes room for itself #3641
Conversation
* Add new `affordForDisplacement` prop (default: true) to allow customization of whether the component makes room for itself by adding padding equivalent to its own height on the document body element. Use case a web app which ordinarily doesn't require vertical scrollbars (with a large enough browser viewport) - the existing unconfigurable behavior was forcing a vertical scrollbar to appear in this scenario, as can be seen in the updated component doc page. * Make clear that the `paddingSize` prop default is "m".
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
jenkins test this |
|
Chromium failed. Jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3641/ |
|
The original reason for adding the padding the the body element is to prevent the overlap of content when the bar exists, for example this scenario: We will then need to provide the consumer an easy way to afford for this in the correct place. We've typically done this through SASS mixins, but since EuiBottomBar can accept any children, it can just about be any size. Therefore, we will need to allow passing/forwarding a |
| * Whether the component should apply padding on the document body element to afford for its own displacement height. | ||
| * Default is true. | ||
| */ | ||
| affordForDisplacement: boolean; |
There was a problem hiding this comment.
Can you add a test for this prop?
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
|
Hey @dannya , Just wondering if you were still going to wrap up this PR or if you need help getting it over the line? I think all it really needs are some tests. |
|
Hi @cchaos , I did start on the tests for this, but wasn't sure of the best approach for testing this specific new functionality (because it would need to test an effect external to the component, in this case the My understanding is that typical Jest tests and snapshots are only testing the rendered component HTML, rather than in the context of a full HTML page (with Any ideas on a way of testing this specific case? Thanks, |
|
Yes, that is ok for now (testing simply the component render). Mainly we want to ensure that the component doesn't error out when using the prop appropriately. We can always circle back to testing of the actual behavior, but for now the simple snapshot is fine. |
|
Hi @dannya I was finishing up this PR for you and attempted to push my changes directly but permissions were denied so I've created a new PR based off this one and going to close this one directly. |
affordForDisplacementprop (default: true) to allow customization of whether the component makes room for itself by adding padding equivalent to its own height on the document body element.Use case is a web app which ordinarily doesn't require vertical scrollbars (with a large enough browser viewport) - the existing unconfigurable behavior was forcing a vertical scrollbar to appear in this scenario, as can be seen in the updated component doc page.
paddingSizeprop default is "m".Summary
The most relevant previous code change this builds off is #680 by @snide
I understand that there is the facility to specify a custom
classNameto effect this, but my rationale here is two-fold - it feels inelegant to have to set a custom CSS class with an!importantrule in order to override the existing behavior (as it is achieved with inline styles), and IMO it is worthwhile having an explicit ability to opt-out of this behavior (whilst keeping the existing behavior as the default).I've seen (quite late into making this change) that there is a similar EuiControlBar component which seems to be favored for future development effort - the same mechanism for automatically adding a height displacement occurs there too. If you are satisfied with my changes here, I will make the same component prop addition in that component, and then update the
CHANGELOG.Checklist